Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Flashbar shadow z-index issue and pages background #2885

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

orangevolon
Copy link
Contributor

@orangevolon orangevolon commented Oct 16, 2024

Description

Reopens #2813 and #2767 after being reverted at #2876, with the following fix:
Flashbar items use box-shadow on the element itself rather than the pseudo element with z-index: -1.

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.21%. Comparing base (cc75463) to head (3a649b9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2885   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files         761      761           
  Lines       21502    21502           
  Branches     7354     7354           
=======================================
  Hits        20688    20688           
  Misses        806      806           
  Partials        8        8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orangevolon orangevolon force-pushed the fix/flashbar-z-index-issue-and-pages-background branch from 7775171 to 172f8ff Compare October 17, 2024 08:09
@orangevolon orangevolon force-pushed the fix/flashbar-z-index-issue-and-pages-background branch from 172f8ff to 1dd11c2 Compare October 18, 2024 07:32
@orangevolon orangevolon changed the title Fix/flashbar z index issue and pages background Fix: Flashbar z index issue and pages background Oct 18, 2024
@orangevolon orangevolon changed the title Fix: Flashbar z index issue and pages background bugfix: Flashbar z index issue and pages background Oct 18, 2024
@orangevolon orangevolon changed the title bugfix: Flashbar z index issue and pages background fix: Flashbar shadow z-index issue and pages background Oct 18, 2024
@orangevolon orangevolon marked this pull request as ready for review October 18, 2024 11:28
@orangevolon orangevolon requested a review from a team as a code owner October 18, 2024 11:28
@orangevolon orangevolon requested review from avinashbot and just-boris and removed request for a team and avinashbot October 18, 2024 11:28
@@ -15,7 +17,9 @@ export default function PageView({ pageId }: { pageId: string }) {
return (
<ErrorBoundary key={pageId}>
<Suspense fallback={<span>Loading...</span>}>
<Page />
<div className={styles['page-container']}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this wrapper? Can we apply the same style to the <div id="app"> that we have already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion, applied the change 👍

@orangevolon orangevolon force-pushed the fix/flashbar-z-index-issue-and-pages-background branch from 1dd11c2 to 7151e3d Compare October 18, 2024 14:08
@orangevolon orangevolon force-pushed the fix/flashbar-z-index-issue-and-pages-background branch from 7151e3d to 3a649b9 Compare October 18, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants